-
Notifications
You must be signed in to change notification settings - Fork 263
[MultiNIC] Propagate SkipDefaultRoutes to CNI and fix windows endpoint default route behavior #4213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@adjordjevic-microsoft please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request addresses multi-NIC scenarios on Windows containers by ensuring the SkipDefaultRoutes flag is properly propagated through the stack and implements a workaround to prevent Windows from implicitly adding unwanted default routes.
Changes:
- Propagates SkipDefaultRoutes field from CNS to CNI in the standalone SwiftV2 flow
- Adds a dummy default route (0.0.0.0/0 via 0.0.0.0) for Windows endpoints when SkipDefaultRoutes is true and no other routes are configured
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cns/restserver/ipam.go | Adds SkipDefaultRoutes field propagation from NC response to PodIpInfo in requestIPConfigHandlerHelperStandalone |
| network/endpoint_windows.go | Implements dummy default route logic to prevent Windows from implicitly adding default routes when SkipDefaultRoutes is enabled |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if epInfo.SkipDefaultRoutes && len(hcnEndpoint.Routes) == 0 { | ||
| logger.Info("Adding dummy default route for SkipDefaultRoutes=true", | ||
| zap.String("endpoint", infraEpName), | ||
| zap.String("nicType", string(epInfo.NICType))) | ||
|
|
||
| hcnEndpoint.Routes = append(hcnEndpoint.Routes, hcn.Route{ | ||
| NextHop: "0.0.0.0", // Dummy gateway | ||
| DestinationPrefix: "0.0.0.0/0", // Default route | ||
| }) | ||
| } |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition len(hcnEndpoint.Routes) == 0 may not cover all multi-NIC scenarios where Windows might add implicit default routes. According to the PR description, this dummy route is added to prevent Windows from implicitly adding a default route when SkipDefaultRoutes is true. However, if epInfo.Routes contains non-default routes (such as specific subnet routes from networkConfig.Routes or networkConfig.CnetAddressSpace), this dummy route won't be added, and Windows might still implicitly add a default route.
Consider whether the condition should be:
if epInfo.SkipDefaultRoutes && !hasDefaultRoute(hcnEndpoint.Routes)- to explicitly check if a default route already exists rather than checking if the route list is empty- Or clarify in a comment why this workaround is only needed when there are no routes at all
Additionally, this new behavior lacks test coverage. Consider adding a test case in endpoint_windows_test.go that verifies the dummy route is added when SkipDefaultRoutes is true and no routes are configured.
Reason for Change:
Change for SkipDefaultRoutes in ipam.go:
SkipDefaultRoutes flag was not fully propagated through the stack. While CNS received the value, it was not forwarded to CNI, and the decision was made at the CNI layer. This change updates requestIPConfigHandlerHelperStandalone to propagate SkipDefaultRoutes from the NC request to CNI, making its behavior consistent with other parameters.
Change for adding dummy route in endpoint_windows.go:
In multi-NIC scenarios on WCOW, CNI may end up adding multiple default routes.
For example - one NIC has SkipDefaultRoutes = false and correctly adds a default route (0.0.0.0/0) via its gateway. Another NIC has SkipDefaultRoutes = true, but Windows still implicitly add an additional default route for it. This results in two default routes, which can negatively impact routing. As a workaround, we explicitly add a dummy default route (0.0.0.0/0) with gateway 0.0.0.0 for the NIC where SkipDefaultRoutes = true, which ensures correct traffic behavior.